Codecov Report

Merging #233 into master will increase coverage by 0.61%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #233      +/-   ##
==========================================
+ Coverage   60.35%   60.96%   +0.61%     
==========================================
  Files         129      129              
  Lines       34677    34728      +51     
  Branches     5862     5894      +32     
==========================================
+ Hits        20929    21172     +243     
+ Misses      13748    13556     -192     

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 489d3f7...7fc2345. Read the comment docs.

• Calling this test wallet-accounts-reveal-test and keeping it in its own file for now

• Running test past the REVEAL point doesn't seem necessary - at that point only one coin owns a name, so there wouldn't be conflicting account issues.

• RPC and HTTP API endpoints are now covered.

Updated to cover sending UPDATE from specified account

@wi-ski Do these changes fix any issues you are seeing?

@tynes - Yes sir they do.

Awesome job @pinheadmz 👏

@tynes @pinheadmz - Dusting off this set of changes :) Is there any chance these are still ok? Happy to take them over the line.

@chjj @boymanjor @pinheadmz @tynes - Can we please get eyes on these changes?

rebase to 5c09c70
rebase on master, update resource serialization used in test.

@pinheadmz - Hey! Ty so much for making these changes <3

I added account support for reveal and redeem, as well as update. I started having second thoughts about how to approach this for owned names: update, register, transfer, finalize, revoke, since once a name is owned, there is only one account that can send the covenant anyway.

I was thinking maybe those calls should just not accept an account argument and when it comes to filling the mtx, those calls should just force a account = null to avoid the "preferred inputs error".

But now I think we should still do this (and I'll have to add 3-4 more commits to this PR) and here's why: a user may want the account that owns a name to ALSO pay the mining fee. This and related coin-selection / privacy between accounts reasons.

SO for update, etc. we will still accept an account parameter but only TWO options are available: the account that owns the name, or null (fee will be paid by any coin in the wallet, and change always goes to default account). If the wrong account is given, that'll be an error.

In the future we should consider adding an account index for bids.

@chjj yeah some users with lots of activity are reporting long processing time generate REVEALs

@chjj @boymanjor @wi-ski
Thanks for bearing with me on this reviewers. I think we're ready for a final pass here. ALL covenant types are now accounted for (we'll need to update the docs to indicate that account is an option). Tests were majorly refactored and simplified.

@boymanjor we have a non-deterministic test in Auction RPC, care to re-run CI build? Everything passes locally for me.

@pinheadmz - Wowza. You're a beast. Thank you for hacking at this.

@chjj's comment inspired me to investigate intro-ing account indices to bid layouts - this: https://github.com/pinheadmz/hsd/pull/2/files

☝️ Is this in the right 'spirit' for adding an account index for bids? Meaning - appending a new int here? If so, can keep pulling that thread.

Im new to LevelDB and the greater-than/less-than mechanics for iterators are not entirely clear to me yet.

That PR is targeting these changes *

@wi-ski I think the idea behind indexing bids is not just adding an integer to the bids in layout, but adding a new index entirely like tx by account but for bids. A PR would also include implantation throughout txdb and wallet to read, write, and search by that index.

Ok - My wishful thinking was wishful. 🙃

@wi-ski I encourage you to continue looking into this problem! The goal would be: when the wallet goes to make a reveal, instead of iterating through every bid in the wallet, it just looks for the bids in the specified account… How could this be accomplished? 🤔

Yes sir, Ill continue poking 🤔

Ty for the guidance